fix(deps): remove unused playwright dependency, clean up install docs#101
fix(deps): remove unused playwright dependency, clean up install docs#101billy-enrizky merged 13 commits intomainfrom
Conversation
…ection Quick Install (curl/PowerShell one-liner) is now the first installation method in both README.md and docs/quickstart.mdx. The standalone "Install Browser" section is removed because OpenBrowser auto-detects system browsers (Chrome, Edge, Brave, Chromium) via LocalBrowserWatchdog._find_installed_browser_path() and auto-installs Chromium on first run if none found. Added Windows PowerShell install to quickstart.mdx Quick Install tab.
Playwright is never imported at runtime -- verified via AST parsing of every .py file in src/openbrowser/. It was only used as a CLI tool for browser installation, but all install paths already use `uvx playwright install` which downloads playwright independently. Saves ~46MB of Node.js driver binaries from pip install.
The install scripts previously fell back to `playwright install chromium` and `python -m playwright install chromium` which required the playwright pip package. Now falls back to `uvx playwright install chromium` which works independently. Simplifies install.ps1 from 3 nested try/catch to 2.
When `openbrowser-ai -c` (no argument) prints the function reference, it also tries to pre-start the daemon. If the daemon start times out, the TimeoutError propagated up and caused exit code 1 even though the help text was already printed. Wrap the best-effort daemon start in try/except so help output always exits 0.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly streamlines the installation process and reduces the core dependency footprint of the OpenBrowser project. By removing an unused Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughReorders installation docs and Homebrew placement, consolidates browser-install messaging (OpenBrowser auto-detects and may install Chromium on first run), prefers Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI
participant Agent as Agent (optional)
participant Daemon as Daemon
participant User as User
CLI->>CLI: parse args (--mcp / prompt / default)
alt --mcp requested
CLI->>CLI: avoid heavy imports, best-effort telemetry
CLI-->>User: start MCP mode (minimal deps)
else prompt requested
CLI->>Agent: attempt guarded import
alt Agent present
CLI->>Daemon: check/start daemon (try/except)
Daemon-->>CLI: running
CLI->>Agent: start agent flow
Agent-->>User: interactive prompt flow
else Agent missing
CLI-->>User: print install-agent-extras guidance and exit
end
else default
CLI-->>User: show simplified help/output (no heavy TUI init)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully removes the unused playwright dependency, which significantly reduces the installation size. The documentation and installation scripts have been updated accordingly to reflect this change, improving the user onboarding experience. I've found a couple of minor areas for improvement in the installation scripts' warning messages to avoid potential user confusion. The fix for the flaky CLI test is also a welcome addition for stability. Overall, this is a solid set of changes.
| catch { | ||
| Write-Warn "Chromium install skipped (run 'openbrowser-ai install' manually)" | ||
| } | ||
| Write-Warn "Chromium install skipped (run 'openbrowser-ai install' after installing uv)" |
There was a problem hiding this comment.
The warning message "run 'openbrowser-ai install' after installing uv" might be confusing for users who don't have uv installed, as the script can also use pipx or pip. A more generic message would be clearer.
Write-Warn "Chromium install skipped. Please run 'openbrowser-ai install' manually after installation completes."
There was a problem hiding this comment.
Fixed in fda5e2d. Updated to: "Chromium install skipped. Please run 'openbrowser-ai install' manually after installation completes."
| uvx playwright install chromium 2>/dev/null || warn "Chromium install failed (run 'openbrowser-ai install' manually)" | ||
| else | ||
| "$PYTHON" -m playwright install chromium 2>/dev/null || warn "Chromium install skipped (run 'openbrowser-ai install' manually)" | ||
| warn "Chromium install skipped (run 'openbrowser-ai install' after installing uv)" |
There was a problem hiding this comment.
The warning message "run 'openbrowser-ai install' after installing uv" could be confusing if the user doesn't have uv installed, since pipx or pip might be used for installation instead. A more general instruction would be more helpful.
| warn "Chromium install skipped (run 'openbrowser-ai install' after installing uv)" | |
| warn "Chromium install skipped. Please run 'openbrowser-ai install' manually after installation completes." |
There was a problem hiding this comment.
Fixed in fda5e2d. Same generic message applied.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openbrowser/cli.py (1)
67-70: Silently ignoring daemon start failures may leave orphaned processes.When
_start_daemon()times out, the subprocess has already been spawned (viasubprocess.Popenwithstart_new_session=Trueinclient.py:54-60) but the socket isn't ready yet. Silently ignoring this means:
- A daemon process continues running in the background
- Subsequent
-ccalls may spawn additional daemons or encounter conflictsConsider logging a debug message on failure so users can diagnose issues, or checking if a daemon is already starting before spawning another:
Suggested improvement
try: asyncio.run(client._start_daemon()) except (TimeoutError, OSError): - pass # Best-effort daemon pre-start; failure is not fatal for help output + pass # Best-effort; daemon may still be starting in backgroundAt minimum, the comment could note that the daemon may continue starting in the background.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openbrowser/cli.py` around lines 67 - 70, The current CLI silently swallows exceptions from client._start_daemon(), which can leave a spawned subprocess (created via subprocess.Popen with start_new_session=True in the daemon startup flow) running; update the except block in the CLI where client._start_daemon() is called to at minimum log a debug/warn message that the daemon may still be starting and include the caught exception, and preferably add a guard (e.g., call into a client.is_starting_or_running() helper or check for an existing socket) before attempting to spawn a new daemon to avoid duplicates; reference client._start_daemon and the subprocess.Popen start_new_session behavior when adding the log and/or the pre-spawn check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/openbrowser/cli.py`:
- Around line 67-70: The current CLI silently swallows exceptions from
client._start_daemon(), which can leave a spawned subprocess (created via
subprocess.Popen with start_new_session=True in the daemon startup flow)
running; update the except block in the CLI where client._start_daemon() is
called to at minimum log a debug/warn message that the daemon may still be
starting and include the caught exception, and preferably add a guard (e.g.,
call into a client.is_starting_or_running() helper or check for an existing
socket) before attempting to spawn a new daemon to avoid duplicates; reference
client._start_daemon and the subprocess.Popen start_new_session behavior when
adding the log and/or the pre-spawn check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7420891d-82ff-4003-aa3f-102be05182d4
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
README.mddocs/quickstart.mdxinstall.ps1install.shpyproject.tomlsrc/openbrowser/cli.py
💤 Files with no reviewable changes (1)
- pyproject.toml
There was a problem hiding this comment.
Pull request overview
This PR removes Playwright as a direct runtime dependency and updates installation/docs to reflect a “no separate browser install” experience, while making the CLI’s daemon pre-start best-effort in the -c (no code) path.
Changes:
- Drop
playwright(and its transitive deps) frompyproject.toml/uv.lock. - Update install scripts + docs to prefer
openbrowser-ai install/uvx playwright install chromiumand message “no separate browser install needed”. - Make daemon pre-start in the
-cno-argument path non-fatal on failures.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Removes Playwright-related packages from the lockfile. |
pyproject.toml |
Drops playwright from core dependencies. |
src/openbrowser/cli.py |
Wraps daemon pre-start with try/except for best-effort output. |
install.sh |
Adjusts Chromium install behavior to use openbrowser-ai install or uvx playwright .... |
install.ps1 |
Same as install.sh for Windows PowerShell. |
README.md |
Updates install/browser-install guidance and reorganizes install sections. |
docs/quickstart.mdx |
Mirrors README guidance in docs quickstart flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| try: | ||
| asyncio.run(client._start_daemon()) | ||
| except (TimeoutError, OSError): | ||
| pass # Best-effort daemon pre-start; failure is not fatal for help output |
There was a problem hiding this comment.
Fixed in fda5e2d. Wrapped both status() and _start_daemon() calls in try/except that catches TimeoutError, OSError, and asyncio.CancelledError. The status() call now returns None on failure, and the logic falls through to the verbose description path.
| # or | ||
| playwright install chromium | ||
| ``` | ||
| > **No separate browser install needed.** OpenBrowser auto-detects any installed Chromium-based browser (Chrome, Edge, Brave, Chromium) and uses it directly. If none is found, Chromium is installed automatically on first run. To pre-install manually: `openbrowser-ai install` |
There was a problem hiding this comment.
Fixed in fda5e2d. Updated both README.md and docs/quickstart.mdx to clarify: "If none is found and uvx is available, Chromium is installed automatically on first run."
| playwright install chromium | ||
| ``` | ||
| <Info> | ||
| **No separate browser install needed.** OpenBrowser auto-detects any installed Chromium-based browser (Chrome, Edge, Brave, Chromium) and uses it directly. If none is found, Chromium is installed automatically on first run. To pre-install manually: `openbrowser-ai install` |
There was a problem hiding this comment.
Fixed in fda5e2d. Same clarification applied to quickstart docs.
| info "Installing Chromium browser..." | ||
| if command -v openbrowser-ai >/dev/null 2>&1; then | ||
| openbrowser-ai install 2>/dev/null || warn "Chromium install failed (run 'openbrowser-ai install' manually)" | ||
| elif command -v playwright >/dev/null 2>&1; then | ||
| playwright install chromium 2>/dev/null || warn "Chromium install failed (run 'playwright install chromium' manually)" | ||
| elif command -v uvx >/dev/null 2>&1; then | ||
| uvx playwright install chromium 2>/dev/null || warn "Chromium install failed (run 'openbrowser-ai install' manually)" | ||
| else | ||
| "$PYTHON" -m playwright install chromium 2>/dev/null || warn "Chromium install skipped (run 'openbrowser-ai install' manually)" | ||
| warn "Chromium install skipped (run 'openbrowser-ai install' after installing uv)" |
There was a problem hiding this comment.
Fixed the warning message in fda5e2d. Now says "Please run 'openbrowser-ai install' manually after installation completes" without mentioning uv specifically. Note that openbrowser-ai install itself shells out to uvx -- this is by design since uv is the recommended installer and most users will have it.
| $null = Get-Command 'uvx' -ErrorAction Stop | ||
| & uvx playwright install chromium 2>$null | ||
| if ($LASTEXITCODE -ne 0) { | ||
| Write-Warn "Chromium install failed (run 'playwright install chromium' manually)" | ||
| Write-Warn "Chromium install failed (run 'openbrowser-ai install' manually)" | ||
| } | ||
| } | ||
| catch { | ||
| try { | ||
| Invoke-Python -Arguments @('-m', 'playwright', 'install', 'chromium') 2>$null | ||
| if ($LASTEXITCODE -ne 0) { | ||
| Write-Warn "Chromium install skipped (run 'openbrowser-ai install' manually)" | ||
| } | ||
| } | ||
| catch { | ||
| Write-Warn "Chromium install skipped (run 'openbrowser-ai install' manually)" | ||
| } | ||
| Write-Warn "Chromium install skipped (run 'openbrowser-ai install' after installing uv)" | ||
| } |
There was a problem hiding this comment.
Fixed in fda5e2d. Same generic warning message applied to install.ps1.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="install.sh">
<violation number="1" location="install.sh:163">
P2: This new fallback skips Chromium installation for successful pip/pipx installs whenever the freshly installed CLI is not yet on PATH. Use a post-install path that doesn't depend on `openbrowser-ai` or `uvx` being immediately available.</violation>
<violation number="2" location="install.sh:163">
P3: Warning message incorrectly tells the user to install `uv` even though the script supports `pipx` and `pip` as install methods. If the user installed via `pip`/`pipx` and just needs a shell restart for PATH to update, this message is misleading. Use a more generic message.</violation>
</file>
<file name="install.ps1">
<violation number="1" location="install.ps1:171">
P3: Warning message incorrectly tells the user to install `uv` even though the script supports `pipx` and `pip` as install methods. If the user installed via `pip`/`pipx` and just needs a shell restart for PATH to update, this message is misleading. Use a more generic message.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| uvx playwright install chromium 2>/dev/null || warn "Chromium install failed (run 'openbrowser-ai install' manually)" | ||
| else | ||
| "$PYTHON" -m playwright install chromium 2>/dev/null || warn "Chromium install skipped (run 'openbrowser-ai install' manually)" | ||
| warn "Chromium install skipped (run 'openbrowser-ai install' after installing uv)" |
There was a problem hiding this comment.
P2: This new fallback skips Chromium installation for successful pip/pipx installs whenever the freshly installed CLI is not yet on PATH. Use a post-install path that doesn't depend on openbrowser-ai or uvx being immediately available.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At install.sh, line 163:
<comment>This new fallback skips Chromium installation for successful pip/pipx installs whenever the freshly installed CLI is not yet on PATH. Use a post-install path that doesn't depend on `openbrowser-ai` or `uvx` being immediately available.</comment>
<file context>
@@ -157,10 +157,10 @@ if [ "$SKIP_BROWSER" = false ]; then
+ uvx playwright install chromium 2>/dev/null || warn "Chromium install failed (run 'openbrowser-ai install' manually)"
else
- "$PYTHON" -m playwright install chromium 2>/dev/null || warn "Chromium install skipped (run 'openbrowser-ai install' manually)"
+ warn "Chromium install skipped (run 'openbrowser-ai install' after installing uv)"
fi
fi
</file context>
There was a problem hiding this comment.
The PATH issue for freshly installed CLIs is pre-existing behavior unrelated to this PR. Fixed the warning message in fda5e2d to use a generic message that doesn't incorrectly tell users to install uv.
There was a problem hiding this comment.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
The bare `openbrowser-ai` command and `-p` flag import Agent/Controller which require the [agent] extra (openai, langgraph, etc). Without the extra, the import crashes with ModuleNotFoundError. Now catches the ImportError and shows a clear message pointing to the correct install command. MCP (-mcp), CLI daemon (-c), and install subcommands continue to work without agent deps.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openbrowser/cli.py`:
- Around line 67-70: The try/except around asyncio.run(client._start_daemon())
only catches TimeoutError and OSError but not asyncio.CancelledError; update the
exception handling for the best-effort daemon pre-start to also catch
asyncio.CancelledError (or use a broader exception like Exception if you prefer)
so that cancellation during await asyncio.sleep(0.2) inside
client._start_daemon() is swallowed and does not cause an unclean exit; modify
the try/except that wraps asyncio.run(client._start_daemon()) to include
asyncio.CancelledError (or Exception) alongside TimeoutError and OSError.
The Textual-based TUI was stuck/non-functional. Removed entirely: - Removed OpenBrowserApp class and all Textual widget code (~1200 lines) - Removed textual_interface() function - Removed [cli] extra (textual dependency) from pyproject.toml - Bare `openbrowser-ai` command now shows help with usage examples - All other modes unaffected: -p (prompt), -c (code), --mcp, init, install
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/openbrowser/cli.py">
<violation number="1" location="src/openbrowser/cli.py:702">
P1: The new default branch removes the no-argument interactive CLI entirely and now only shows help.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| traceback.print_exc() | ||
| sys.exit(1) | ||
| # No specific mode requested -- show help | ||
| click.echo(ctx.get_help()) |
There was a problem hiding this comment.
P1: The new default branch removes the no-argument interactive CLI entirely and now only shows help.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/openbrowser/cli.py, line 702:
<comment>The new default branch removes the no-argument interactive CLI entirely and now only shows help.</comment>
<file context>
@@ -1955,92 +698,8 @@ def run_main_interface(ctx: click.Context, debug: bool = False, **kwargs):
- traceback.print_exc()
- sys.exit(1)
+ # No specific mode requested -- show help
+ click.echo(ctx.get_help())
</file context>
There was a problem hiding this comment.
This is intentional. The Textual TUI was non-functional (stuck on launch, never rendered properly). It was removed per project owner's request. The bare command now shows help with usage examples for the working modes: -p (prompt), -c (code execution), --mcp (MCP server), init, and install.
There was a problem hiding this comment.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
- Catch asyncio.CancelledError in daemon pre-start and status check - Wrap status() call in try/except for robustness - Fix misleading warning in install.sh/install.ps1 (don't mention uv specifically) - Clarify uvx requirement for auto-install in README and quickstart docs
There was a problem hiding this comment.
3 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/openbrowser/cli.py">
<violation number="1" location="src/openbrowser/cli.py:62">
P2: Don't treat a `status()` timeout as "daemon not running". A read timeout can happen while the daemon is still alive, so this path prints the cold-start help and tries to start a second daemon.
(Based on your team's feedback about excluding asyncio.TimeoutError from daemon auto-restart conditions.) [FEEDBACK_USED]</violation>
</file>
<file name="docs/quickstart.mdx">
<violation number="1" location="docs/quickstart.mdx:97">
P2: `openbrowser-ai install` is documented as a universal fallback, but it also requires `uvx`, so pip/pipx users without uv still hit a broken browser-install path.</violation>
</file>
<file name="README.md">
<violation number="1" location="README.md:140">
P2: `openbrowser-ai install` is documented here as the manual fallback, but that command also requires `uvx`, so pip/pipx users without `uv` still have no working browser-install path.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 140: Update the README sentence about manual pre-install to state the
prerequisite: clarify that the `openbrowser-ai install` command is available
only if the `uvx` tool is present, e.g., change the sentence mentioning "To
pre-install manually: `openbrowser-ai install`" to explicitly note that this
install path requires `uvx` to be installed/available on the system so users
aren’t misled; reference `openbrowser-ai install` and `uvx` in the same sentence
for clarity.
In `@src/openbrowser/cli.py`:
- Line 637: Update the click option help for the '-p/--prompt' flag to remove
the misleading "in headless mode" claim and use a neutral description; locate
the click.option declaration for '-p', '--prompt' (the CLI flag definition in
src/openbrowser/cli.py) and change its help string to something like "Run a
single task using the prompt interface" or "Run a single task (behavior may vary
by configuration)" so it doesn't assert headless behavior that is actually
controlled elsewhere.
- Around line 60-63: The best-effort probe calling asyncio.run(client.status())
may raise JSON parsing errors; update the exception handling around that call
(the try/except that assigns status) to also catch json.JSONDecodeError and
ValueError so parsing failures from client.status() are swallowed and status
simply becomes None; ensure the except clause includes these exception types
alongside TimeoutError, OSError, and asyncio.CancelledError.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 95c2c54a-1f40-45b5-86e5-d7c6abf83d33
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
README.mddocs/quickstart.mdxinstall.ps1install.shpyproject.tomlsrc/openbrowser/cli.py
🚧 Files skipped from review as they are similar to previous changes (3)
- install.sh
- docs/quickstart.mdx
- install.ps1
The pre-flight CDP check called _recover_browser_session() even when browser_session was None (never created), causing it to launch a real browser. In CI (no browser available), this hangs indefinitely. Only attempt recovery when browser_session was previously set (i.e. the browser died), not when it was never created. Fixes CI test hang at test_mcp_server.py::TestExecuteCode.
- Separate TimeoutError handling in daemon status probe (daemon alive but slow should show compact help, not verbose) - Add ValueError to exception handler for malformed daemon JSON - Change -p help text from "headless mode" to "one-shot mode" - Add "(requires uvx)" to manual pre-install note in README and docs
server.py does not import DAEMON_DIR (uses get_socket_path/get_pid_path helpers instead). The env var OPENBROWSER_SOCKET already drives the correct socket/pid paths in tests.
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="README.md">
<violation number="1" location="README.md:140">
P2: The manual browser pre-install command is wrong for the documented `uvx` flow; `openbrowser-ai install` won't exist unless the package was installed separately.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| # or | ||
| playwright install chromium | ||
| ``` | ||
| > **No separate browser install needed.** OpenBrowser auto-detects any installed Chromium-based browser (Chrome, Edge, Brave, Chromium) and uses it directly. If none is found and `uvx` is available, Chromium is installed automatically on first run. To pre-install manually (requires `uvx`): `openbrowser-ai install` |
There was a problem hiding this comment.
P2: The manual browser pre-install command is wrong for the documented uvx flow; openbrowser-ai install won't exist unless the package was installed separately.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 140:
<comment>The manual browser pre-install command is wrong for the documented `uvx` flow; `openbrowser-ai install` won't exist unless the package was installed separately.</comment>
<file context>
@@ -137,7 +137,7 @@ pip install openbrowser-ai[azure] # Azure OpenAI
-> No separate browser install needed. OpenBrowser auto-detects any installed Chromium-based browser (Chrome, Edge, Brave, Chromium) and uses it directly. If none is found and uvx is available, Chromium is installed automatically on first run. To pre-install manually: openbrowser-ai install
+> No separate browser install needed. OpenBrowser auto-detects any installed Chromium-based browser (Chrome, Edge, Brave, Chromium) and uses it directly. If none is found and uvx is available, Chromium is installed automatically on first run. To pre-install manually (requires uvx): openbrowser-ai install
Quick Start
</file context>
</details>
```suggestion
> **No separate browser install needed.** OpenBrowser auto-detects any installed Chromium-based browser (Chrome, Edge, Brave, Chromium) and uses it directly. If none is found and `uvx` is available, Chromium is installed automatically on first run. To pre-install manually (requires `uvx`): `uvx openbrowser-ai install`
add_signal_handler raises RuntimeError when called from a non-main thread. Catch it alongside NotImplementedError (Windows) so the daemon can be embedded in background threads for testing or embedding.
The test previously relied on auto-starting a real daemon which tries to launch a browser -- causing a 30s timeout in CI. Now starts a mock daemon with a pre-initialized executor on a temp socket, then passes OPENBROWSER_SOCKET to the CLI subprocess.
Use pytest.importorskip for ChatAnthropic, ChatOpenAI, ChatGoogle, and ChatBrowserUse tests so they skip gracefully in envs without the [agent] extra instead of failing with ImportError.
There was a problem hiding this comment.
3 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/test_advanced_features.py">
<violation number="1" location="tests/test_advanced_features.py:181">
P2: Don't skip `ChatBrowserUse` behind `litellm`; that package is unrelated to this import, so the test now false-skips and can miss breakages.</violation>
</file>
<file name="tests/test_cli_c_help.py">
<violation number="1" location="tests/test_cli_c_help.py:109">
P3: Close this background event loop after the daemon thread exits to avoid leaking an unclosed loop in the test process.</violation>
<violation number="2" location="tests/test_cli_c_help.py:129">
P2: Schedule the daemon stop event on its event loop instead of calling `asyncio.Event.set()` from the test thread.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| def test_chat_browser_use_import(self): | ||
| """Test ChatBrowserUse import.""" | ||
| pytest.importorskip("litellm") |
There was a problem hiding this comment.
P2: Don't skip ChatBrowserUse behind litellm; that package is unrelated to this import, so the test now false-skips and can miss breakages.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_advanced_features.py, line 181:
<comment>Don't skip `ChatBrowserUse` behind `litellm`; that package is unrelated to this import, so the test now false-skips and can miss breakages.</comment>
<file context>
@@ -178,24 +178,28 @@ class TestNewLLMProviders:
def test_chat_browser_use_import(self):
"""Test ChatBrowserUse import."""
+ pytest.importorskip("litellm")
from openbrowser.llm import ChatBrowserUse
</file context>
| time.sleep(0.05) | ||
| else: | ||
| server._running = False | ||
| server._stop_event.set() |
There was a problem hiding this comment.
P2: Schedule the daemon stop event on its event loop instead of calling asyncio.Event.set() from the test thread.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_cli_c_help.py, line 129:
<comment>Schedule the daemon stop event on its event loop instead of calling `asyncio.Event.set()` from the test thread.</comment>
<file context>
@@ -78,7 +86,68 @@ def test_c_no_arg_compact_when_daemon_running(self):
+ time.sleep(0.05)
+ else:
+ server._running = False
+ server._stop_event.set()
+ t.join(timeout=5)
+ pytest.fail('Mock daemon socket never appeared')
</file context>
| server._stop_event.set() | |
| loop.call_soon_threadsafe(server._stop_event.set) |
| }) | ||
| server._executor = executor | ||
|
|
||
| loop = asyncio.new_event_loop() |
There was a problem hiding this comment.
P3: Close this background event loop after the daemon thread exits to avoid leaking an unclosed loop in the test process.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_cli_c_help.py, line 109:
<comment>Close this background event loop after the daemon thread exits to avoid leaking an unclosed loop in the test process.</comment>
<file context>
@@ -78,7 +86,68 @@ def test_c_no_arg_compact_when_daemon_running(self):
+ })
+ server._executor = executor
+
+ loop = asyncio.new_event_loop()
+
+ def run_daemon():
</file context>
- Catch asyncio.CancelledError in daemon pre-start and status check - Wrap status() call in try/except for robustness - Fix misleading warning in install.sh/install.ps1 (don't mention uv specifically) - Clarify uvx requirement for auto-install in README and quickstart docs
fix(deps): remove unused playwright dependency, clean up install docs
Summary
playwrightfrom base dependencies -- saves ~46MB per install. Playwright was never imported at runtime (verified via AST parsing of every.pyfile). All browser install paths useuvx playwright installwhich downloads playwright independently.openbrowser-aicommand now shows help with usage examples. Removed[cli]extra (textualdependency) from pyproject.toml.LocalBrowserWatchdog._find_installed_browser_path()and auto-installs Chromium on first run if none found.openbrowser-ai -c(no argument) now handles daemon auto-start timeout gracefully instead of propagating TimeoutError as exit code 1.-pmode now show a clear install hint instead of crashing with ImportError.python -m playwright install(requires pip dep) withuvx playwright install(independent).Commits
ff4fc8546b4c96playwrightfrom base dependenciesabb1b031c130b4218e8fd823c753Test plan
.pyinsrc/openbrowser/-- zero playwright importsuv syncremoves playwright from venv -- confirmedfind_spec('playwright') is Nonetest_cli_c_help)openbrowser-aishows help text (no crash, no TUI hang)openbrowser-ai --versionworks-pmode with missing agent deps shows clear error messagetest_c_no_arg_verbose_when_daemon_not_runningnow passes reliablySummary by CodeRabbit
Documentation
Improvements
Bug Fixes
Chores